fix(maven): do not track registries when fetching requirements #1466
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1466 +/- ##
==========================================
- Coverage 67.32% 67.08% -0.24%
==========================================
Files 192 192
Lines 18161 18164 +3
==========================================
- Hits 12226 12185 -41
- Misses 5283 5324 +41
- Partials 652 655 +3 ☔ View full report in Codecov by Sentry. |
cuixq
changed the title
oliverchang
approved these changes
Jan 5, 2025
| @@ -132,7 +132,8 @@ func (e Extractor) Extract(ctx context.Context, input *filesystem.ScanInput) ([] | |||
| } | |||
| overrideClient.AddVersion(root, reqs) | |||
|
|
|||
| client.PreFetch(ctx, overrideClient, reqs, filepath.Join(input.Root, input.Path)) | |||
| // TODO: only run `PreFetch` for deps.dev client | |||
| // client.PreFetch(ctx, overrideClient, reqs, filepath.Join(input.Root, input.Path)) | |||
There was a problem hiding this comment.
what's the impact on performance of this?
There was a problem hiding this comment.
this depends on which registry client we are using, for this pom.xml:
- with deps.dev client: the performance is worse, with runtime from 20s to 1m
- with native registry: the runtime improved from more than 10m to 10s
for reference, it takes mvn about 25s to resolve without any local cache.
michaelkedar
approved these changes
Jan 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#1447
When fetching requirements of a Maven dependency, we should not keep track of the registries defined in pom.xml of dependencies. Doing that will add a lot irrelevant registries to the client and sends unnecessary requests, and finally slows down the resolution.
This PR also disables
PreFetchin Maven transitive scanning.PreFetchcallsMatchVersionswhich sends requests tomaven-metadata.xmlfor native Maven client.maven-metadata.xmlis not necessarily needed for all dependencies. DisablingPreFetchwill save us from making requests to these files. However, callingPreFetchshould be fine for deps.dev client, and this can be a TODO in the future.With this fix, resolving pom.xml is now shortened to 10-20 seconds.